Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implemented stable image shrink #513

Merged
merged 8 commits into from
Feb 15, 2024

Conversation

egordidenko
Copy link

Description

Checklist

// Path: packages/image-shrink/src/helper
export { memoize, memoKeySerializer } from './helper/memoize'

export { shrinkFile, type TSetting } from './utils/shrinkFile'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only thing we should export is this one.

@@ -0,0 +1,4 @@
export const allowLayers = [
Copy link
Member

@nd0ut nd0ut Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in the folder name (constans)

}

const squareSupported = await squareTest(testSquareSide, testSquareSide)
const dimensionSupported = await dimensionTest(testDimension, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably it's better to use Promise.all here


return isTestPass
} catch (e) {
new Error(`Failed to test for max canvas size of ${width}x${height}.`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error isn't throwed or returned. Probably it should be console.log or console.error

return new Promise((resolve, reject) => {
replaceIccProfile(inputFile, [])
.then((file: Blob) => {
console.log({ file }, typeof file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove console log

image: File | Blob | string
): Promise<HTMLImageElement> => {
if (image.src) {
return processImage(image)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove if block

import { imageLoader } from '../image/imageLoader'

export const stripIccProfile = (inputFile: File): Promise<HTMLImageElement> => {
return new Promise((resolve, reject) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactor promises chain

@egordidenko egordidenko force-pushed the feature/implement-image-shrink branch from e5322b6 to de4850d Compare February 15, 2024 12:12
@egordidenko egordidenko merged commit 6c690b3 into master Feb 15, 2024
3 checks passed
@egordidenko egordidenko deleted the feature/implement-image-shrink branch February 15, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants